feat: implement process.cpuUsage (Deno.cpuUsage)
#27217
Conversation
|
|
|
Does it need to check |
|
|
Do I need to change something after adding an op? After add the |
|
The operations are listed in 2 different arrays, and they must have the same order: See Look at the last 2 elements of both lists. |
|
@irbull can you please review again? |
I'm not on the core team so I can't actually review this, but I'll look at this again tomorrow and leave my thoughts, and I can ask on Discord for someone on the team to take a final look. |
| user: 0, | ||
| system: 0, | ||
| }; | ||
| return Deno.cpuUsage(); |
There was a problem hiding this comment.
Small nit, but maybe define this the same way that memoryUsage is defined. That is, create a function and then just Process.prototype.memoryUsage = memoryUsage;.
I was going to suggest moving this next to memoryUsage too, but it seems that the prototypes are mostly organized in alphabetical order (not all), so maybe leave it here.
runtime/ops/os/mod.rs
Outdated
| external: usize, | ||
| #[op2] | ||
| #[serde] | ||
| fn op_runtime_cpu_usage() -> (usize, usize) { |
There was a problem hiding this comment.
Most of the other ops return a Struct, not just a tuple. While a tuple may be faster (I'm not sure how much performance matters here) consistency might be better.
runtime/ops/os/mod.rs
Outdated
| s.external_memory(), | ||
| ); | ||
|
|
||
| (rss, heap_total, heap_used, external) |
There was a problem hiding this comment.
Oh, I see you changed the memory_usage one too. Consistency FTW!
|
@CyanChanges Can you rebase? I tried to do it but don't have write access to your branch (probably because it is |
Wait me a moment to test if it works as expected. |
CyanChanges
force-pushed
the
main
branch
5 times, most recently
from
ff91c2d to
d6e5469
Compare
|
@littledivy All good now. |
No description provided.